Documentation Review

Summary

This PR fixes the external-links.ts script to treat subdomains of pulumi.com as internal links, preventing them from opening in new tabs. The changes are clean and focused.

Issues Found

1. Logic correctness concern - theme/src/ts/external-links.ts:30-31

The subdomain check logic may not work correctly for all cases. Consider these scenarios:

• When on pulumi.com (currentDomain = pulumi.com):

  • Link to app.pulumi.com (linkDomain = app.pulumi.com): ✅ Works (app.pulumi.com ends with .pulumi.com)
  • Link to www.pulumi.com (linkDomain = pulumi.com after www removal): ✅ Works (exact match)

• When on app.pulumi.com (currentDomain = app.pulumi.com):

  • Link to pulumi.com (linkDomain = pulumi.com): ✅ Works (app.pulumi.com ends with .pulumi.com)
  • Link to registry.pulumi.com (linkDomain = registry.pulumi.com): ✅ Works (both end with .pulumi.com)

However, the current logic has a potential edge case issue:

• When on example.com (currentDomain = example.com):
  • Link to notexample.com (linkDomain = notexample.com): ✅ Correctly external

The third condition (currentDomain.endsWith('.' + linkDomain)) could theoretically match unintended domains if someone creates pulumi.com.evil.com, but this is unlikely in practice since you control the deployment domains.

Suggestion: Consider using a more explicit base domain check:

// Extract base domain (last two parts for most TLDs)
const getBaseDomain = (domain: string): string => {
    const parts = domain.split('.');
    return parts.length >= 2 ? parts.slice(-2).join('.') : domain;
};

const linkBaseDomain = getBaseDomain(linkDomain);
const currentBaseDomain = getBaseDomain(currentDomain);
const isInternal = linkBaseDomain === currentBaseDomain;

This would treat pulumi.com, app.pulumi.com, registry.pulumi.com, and www.pulumi.com all as internal.

2. Code style - theme/src/ts/external-links.ts:29-31

The multi-line boolean expression could be more readable with parentheses or consistent formatting, though the current approach is acceptable.

Positive Notes

• The fix correctly addresses the issue described in the PR
• Code changes are minimal and focused
• The logic handles the bidirectional subdomain relationship well
• Comment update clearly explains the new behavior

Need additional reviews or fixes? Mention @claude and I'll help out!

Your site preview for commit 3cca078 is ready! 🎉

http://www-testing-pulumi-docs-origin-pr-16839-3cca078b.s3-website.us-west-2.amazonaws.com.

@asafashirov I think you need to run make build-assets at the cli to regenerate the js bundle, for the external-links.ts changes to work.

Your site preview for commit a5d327d is ready! 🎉

http://www-testing-pulumi-docs-origin-pr-16839-a5d327dc.s3-website.us-west-2.amazonaws.com.

@adamgordonbell I just tried that - it didn't work.

You can see in the preview that it's still opening a new tab on homepage. I believe that is because the builded js hasn't been updated.

@adamgordonbell that didn't seem to work on my end. My Claude Code thinks "the CI pipeline may need to clear its Hugo cache or ensure make build-assets runs after pulling the latest code. This is a CI caching issue, not a code issue." but I have not idea how to come about doing this.